Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update tag commands in gmpv8 and exclude tests from installation #115

Merged
merged 5 commits into from
Apr 23, 2019

Conversation

qha
Copy link
Contributor

@qha qha commented Apr 19, 2019

Implement create_tag and modify_tag in gvm.protocols.gmpv8 with parameters and sent command structure according to the api documentation.

Exclude tests from installation.

The first commit concerning gvm.protocols.gmpv8:create_tag has actually been tested against a gvm, the rest are armchair exercises. I will be happy to create a new merge request with only tested code (though i don't anticipate a need for modify_tag in my projects) or to squash the commits if you prefer.

Checklist:

@qha qha requested a review from a team April 19, 2019 13:13
@qha qha force-pushed the update-tag-commands-in-gmpv8 branch from fa8465b to 9924f92 Compare April 19, 2019 13:19
@codecov
Copy link

codecov bot commented Apr 19, 2019

Codecov Report

Merging #115 into master will increase coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #115      +/-   ##
=========================================
+ Coverage   96.33%   96.4%   +0.07%     
=========================================
  Files          10      10              
  Lines        2428    2479      +51     
=========================================
+ Hits         2339    2390      +51     
  Misses         89      89
Impacted Files Coverage Δ
gvm/protocols/gmpv7.py 99.74% <ø> (ø) ⬆️
gvm/protocols/gmpv8.py 99.36% <100%> (+0.3%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1749a55...5530cf6. Read the comment docs.

@qha qha force-pushed the update-tag-commands-in-gmpv8 branch from 9924f92 to 979dedc Compare April 19, 2019 13:27
@qha qha force-pushed the update-tag-commands-in-gmpv8 branch from 979dedc to f6c1a7a Compare April 19, 2019 13:36
Copy link
Contributor

@bjoernricks bjoernricks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot! Could you remove 1829bdd and f6c1a7a from this PR too? They are unrelated and shouldn't be included here.

The other stuff is only nitpicking and nice to have.

gvm/protocols/gmpv8.py Outdated Show resolved Hide resolved
gvm/protocols/gmpv8.py Outdated Show resolved Hide resolved
gvm/protocols/gmpv8.py Outdated Show resolved Hide resolved
gvm/protocols/gmpv8.py Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
gvm/protocols/gmpv8.py Show resolved Hide resolved
gvm/protocols/gmpv8.py Show resolved Hide resolved
gvm/protocols/gmpv8.py Show resolved Hide resolved
gvm/protocols/gmpv8.py Show resolved Hide resolved
gvm/protocols/gmpv8.py Show resolved Hide resolved
@qha qha force-pushed the update-tag-commands-in-gmpv8 branch from f6c1a7a to 9694136 Compare April 23, 2019 12:31
@qha qha force-pushed the update-tag-commands-in-gmpv8 branch from 9694136 to d73b1aa Compare April 23, 2019 12:35
@qha
Copy link
Contributor Author

qha commented Apr 23, 2019

Thanks a lot! Could you remove 1829bdd and f6c1a7a from this PR too? They are unrelated and shouldn't be included here.

Ok, sure. Do you still want them in another pr or possibly two new prs?

The other stuff is only nitpicking and nice to have.

I think i've fixed that to, please have another look.

@bjoernricks
Copy link
Contributor

I would apply 1829bdd if Gmpv8 is changed too.

Regarding the tests, I am not sure if they should be excluded from the wheel completely but installing them as tests package seems bad to me. So I would go for it too.

Feel free to create on PR for both changes.

@bjoernricks bjoernricks merged commit 1c7c93e into greenbone:master Apr 23, 2019
@qha qha deleted the update-tag-commands-in-gmpv8 branch April 24, 2019 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants